Skip to content

Conversation

@samhvw8
Copy link
Contributor

@samhvw8 samhvw8 commented Mar 7, 2025

Context

There is many case that user want Roo read sample of file or read file in range of line

Implementation

Screenshots

before after

How to Test

Get in Touch


Important

Adds read_file_range tool to read specific line ranges from files, with updated tool descriptions and tests.

  • Behavior:
    • Adds read_file_range tool to Cline class in Cline.ts for reading specific line ranges from files.
    • Updates toolUseNames and toolParamNames in index.ts to include read_file_range.
    • Adds getReadFileRangeDescription() in read-file-range.ts for tool description.
  • Tests:
    • Adds tests for readLines() in nthline.test.ts to validate line range reading, including edge cases like negative and non-integer line numbers.
  • Misc:
    • Implements readLines() function in nthline.ts to read specified line ranges from files.
    • Updates TOOL_DISPLAY_NAMES and TOOL_GROUPS in tool-groups.ts to include read_file_range.

This description was created by Ellipsis for 9099f87199323125e269c58b2289adde4e74a771. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented Mar 7, 2025

⚠️ No Changeset found

Latest commit: 3e22c4d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Mar 7, 2025
@samhvw8 samhvw8 force-pushed the feat/read_file_range branch 2 times, most recently from f4d064a to 673f11b Compare March 7, 2025 09:32
@hannesrudolph hannesrudolph moved this from New to PR [Greenlit] in Roo Code Roadmap Mar 7, 2025
@mrubens
Copy link
Collaborator

mrubens commented Mar 9, 2025

Instead of adding a new tool and having the LLM choose between them, should we just add optional start/end line to the read_file tool along with a setting for the maximum number of lines to read from a file?

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 9, 2025

Instead of adding a new tool and having the LLM choose between them, should we just add optional start/end line to the read_file tool along with a setting for the maximum number of lines to read from a file?

ok, i will update pr with that approach

@samhvw8 samhvw8 force-pushed the feat/read_file_range branch from 6171fc4 to afbc064 Compare March 9, 2025 17:23
@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 9, 2025

@mrubens updated ! :D

@samhvw8 samhvw8 force-pushed the feat/read_file_range branch 3 times, most recently from fc86b8f to 2008a5e Compare March 10, 2025 00:02
@mrubens
Copy link
Collaborator

mrubens commented Mar 10, 2025

Nice! How does it feel?

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 10, 2025

@mrubens it's work perfectly :D current test with file 333M (2000000 line) from https://www.datablist.com/learn/csv/download-sample-csv-files, and it work ok, i think we can support mention read file in range with syntax like "@test.log::{10,100}"

image

@samhvw8 samhvw8 force-pushed the feat/read_file_range branch from 2008a5e to d50f41c Compare March 10, 2025 03:42
@mrubens
Copy link
Collaborator

mrubens commented Mar 12, 2025

A couple thoughts on this:

  1. I think we should have a default (that can be configured in settings) where if you don't specific the start and end it reads up to X lines of the file. I think that would save a lot of tokens from accidentally reading long files.
  2. Along with that, I think we would need to return more information about the file along with the selected range of lines. Specifically how many total lines there are in the file, and ideally information from the tree-sitter about the definitions within the file and which lines they're on (similar to what Codey does).

What do you think?

@mrubens
Copy link
Collaborator

mrubens commented Mar 12, 2025

I guess not exactly the same approach, but this is what I'm thinking of: https://github.com/ccrvlh/codey/blob/f823be88322b7417d75c4db98a0cc224de6ad97b/src/core/tools.ts#L306-L341

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 13, 2025

A couple thoughts on this:

  1. I think we should have a default (that can be configured in settings) where if you don't specific the start and end it reads up to X lines of the file. I think that would save a lot of tokens from accidentally reading long files.
  2. Along with that, I think we would need to return more information about the file along with the selected range of lines. Specifically how many total lines there are in the file, and ideally information from the tree-sitter about the definitions within the file and which lines they're on (similar to what Codey does).

What do you think?

  1. sound good idea, let me do that
  2. let me thinking how can we do it

@mrubens
Copy link
Collaborator

mrubens commented Mar 13, 2025

Great! Maybe we can reuse what that list_code_definitions tool does somehow?

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 13, 2025

Great! Maybe we can reuse what that list_code_definitions tool does somehow?

yes, but the tool use processing is hard to maintain now, we need to separate into another module to easy maintain later

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Mar 13, 2025
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the default should be 500

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er, let's wait to make a call - but 4500 feels huge

@mrubens
Copy link
Collaborator

mrubens commented Mar 14, 2025

Does tail behavior work? for example:

Reading the lines >= 1000 of a log file:

<read_file>
<path>logs/application.log</path>
<start_line>1000</start_line>
</read_file>

I think so? But maybe we would actually want to support start_line=-100 for this (100 from the end)?

@KJ7LNW
Copy link
Contributor

KJ7LNW commented Mar 14, 2025

this approach would allow multiple ranges, slicing:

<range>123:456</range>
<range>777:</range>       # 777 until the end 
<range>:1111</range>     # beginning until 1111 (inclusive) 

also see my related comment here: #1374 (comment)

I think so? But maybe we would actually want to support start_line=-100 for this (100 from the end)?

I like that notation too, perhaps:

<range>:-100</range>

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 15, 2025

this approach would allow multiple ranges, slicing:

<range>123:456</range>
<range>777:</range>       # 777 until the end 
<range>:1111</range>     # beginning until 1111 (inclusive) 

also see my related comment here: #1374 (comment)

I think so? But maybe we would actually want to support start_line=-100 for this (100 from the end)?

I like that notation too, perhaps:

<range>:-100</range>

@KJ7LNW sound good idea

@mrubens should we allow read multiple file but limit their range ?

@KJ7LNW
Copy link
Contributor

KJ7LNW commented Mar 15, 2025

@mrubens should we allow read multiple file but limit their range ?

what do you mean by "allow read multiple file" ?

but limit their range

hard-coded limits are a bad idea. let the AI choose, or create a config option for limits

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 15, 2025

@mrubens should we allow read multiple file but limit their range ?

what do you mean by "allow read multiple file" ?

in single request and response allow user can read multiple file

but limit their range

hard-coded limits are a bad idea. let the AI choose, or create a config option for limits

No we have setting, and already have that in this PR, you can check it

@mrubens
Copy link
Collaborator

mrubens commented Mar 15, 2025

I like the ideas of the support for tailing and multiple file reads, but I think those can be follow-ups - there's already a lot in this PR.

@samhvw8 samhvw8 force-pushed the feat/read_file_range branch 3 times, most recently from 00332e4 to 890f237 Compare March 19, 2025 04:00
@KJ7LNW
Copy link
Contributor

KJ7LNW commented Mar 19, 2025

it looks like this PR picked up a huge number of changes that are not related to this feature.

can you rebase on main?

be sure to backup your work first:

  • git brach read-file-range-before-rebase
  • git fetch origin # assuming roo is origin
  • git rebase origin/main

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 19, 2025

it looks like this PR picked up a huge number of changes that are not related to this feature.

can you rebase on main?

be sure to backup your work first:

  • git brach read-file-range-before-rebase

  • git fetch origin # assuming roo is origin

  • git rebase origin/main

Already rebase on main, you can check the git commit

@KJ7LNW
Copy link
Contributor

KJ7LNW commented Mar 19, 2025

it looks like this PR picked up a huge number of changes that are not related to this feature.
can you rebase on main?
be sure to backup your work first:

  • git brach read-file-range-before-rebase
  • git fetch origin # assuming roo is origin
  • git rebase origin/main

Already rebase on main, you can check the git commit

that looks much better thank you!

@samhvw8 samhvw8 force-pushed the feat/read_file_range branch from 890f237 to d0143c9 Compare March 19, 2025 10:06
feat: add file truncation with code structure preview

Add configurable line limit for large file handling with source code structure preview. This improves performance and memory usage while maintaining code readability.

Key changes:

Add maxReadFileLine setting (default: 4500) with UI controls
Create efficient line counter utility using streams
Add parseSourceCodeDefinitionsForFile for single file parsing
Show truncation notice with code structure for large files
Add comprehensive tests for new functionality
Move file extensions to shared constant

add line number

pr comment

Return definition ranges from tree-sitter

make treesitter always add line

add line number to rg search

enhance tree siter output & fix test

update snapshot

fix test
@samhvw8 samhvw8 force-pushed the feat/read_file_range branch from c27b6e4 to 3e1f7b6 Compare March 19, 2025 11:46
@mrubens
Copy link
Collaborator

mrubens commented Mar 19, 2025

Going to run this as my editor today and see how it goes!

@KJ7LNW
Copy link
Contributor

KJ7LNW commented Mar 19, 2025

Going to run this as my editor today and see how it goes!

I am curious how the AI utilizes the file ranges!

@mrubens
Copy link
Collaborator

mrubens commented Mar 20, 2025

@samhvw8 looking at the exports of my chats, I don't think it's currently appending the definitions to the tool result when the read is truncated.

@samhvw8
Copy link
Contributor Author

samhvw8 commented Mar 20, 2025

@samhvw8 looking at the exports of my chats, I don't think it's currently appending the definitions to the tool result when the read is truncated.

Let me find a way to truncate more 💪🏻

@mrubens
Copy link
Collaborator

mrubens commented Mar 20, 2025

@samhvw8 looking at the exports of my chats, I don't think it's currently appending the definitions to the tool result when the read is truncated.

Let me find a way to truncate more 💪🏻

Oh I think I was confused!

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Mar 20, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 20, 2025
@mrubens mrubens merged commit 70a476e into RooCodeInc:main Mar 20, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from PR [Greenlit] to Done in Roo Code Roadmap Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants